Skip to content

use File.pathSeparator for classpath in genDocs #4182

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 26, 2018

Conversation

robstoll
Copy link
Contributor

  • uses File.pathSeparator instead of hard-coded path separator :
    in genDocs => this way Windows user can run the command as well

Copy link
Member

@dottybot dottybot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

@smarter
Copy link
Member

smarter commented Mar 25, 2018

There are other uses of ":" in Build.scala, would you mind fixing them too?

@robstoll robstoll force-pushed the genDocs-path-separator branch from e9b5a97 to 51b9042 Compare March 25, 2018 19:53
@robstoll
Copy link
Contributor Author

@robstoll robstoll force-pushed the genDocs-path-separator branch 6 times, most recently from 4236369 to 4e1042d Compare March 25, 2018 20:54
@robstoll
Copy link
Contributor Author

An improvement would be to reduce code duplication between genDocs and dottydoc, lines 366-370 and 388-391 are the same. This is my first encounter with sbt and I don't know how to move code into an own function without encountering the error that I cannot access value outside of a task macro definition.
Would be great if someone could do that (surely an easy task) so that I know how to do it next time :)

@allanrenucci
Copy link
Contributor

allanrenucci commented Mar 26, 2018

Would be great if someone could do that (surely an easy task) so that I know how to do it next time :)

Probably something like this:

diff --git a/project/Build.scala b/project/Build.scala
index 4d7f1ad1d..df0b5e13d 100644
--- a/project/Build.scala
+++ b/project/Build.scala
@@ -345,6 +345,13 @@ object Build {
       javacOptions in (Compile, doc) --= Seq("-Xlint:unchecked", "-Xlint:deprecation")
     )
 
+  private lazy val dottydocClasspath = Def.task {
+    val dottyLib = (packageAll in `dotty-compiler`).value("dotty-library")
+    val dottyInterfaces = (packageAll in `dotty-compiler`).value("dotty-interfaces")
+    val otherDeps = (dependencyClasspath in Compile).value.map(_.data).mkString(File.pathSeparator)
+    dottyLib + File.pathSeparator + dottyInterfaces + File.pathSeparator + otherDeps
+  }
+
   // Settings shared between dotty-doc and dotty-doc-bootstrapped
   lazy val dottyDocSettings = Seq(
     baseDirectory in (Compile, run) := baseDirectory.value / "..",
@@ -364,9 +371,6 @@ object Build {
       val majorVersion = baseVersion.take(baseVersion.lastIndexOf('.'))
       IO.write(file("./docs/_site/versions/latest-nightly-base"), majorVersion)
 
-      val dottyLib = (packageAll in `dotty-compiler`).value("dotty-library")
-      val dottyInterfaces = (packageAll in `dotty-compiler`).value("dotty-interfaces")
-      val otherDeps = (dependencyClasspath in Compile).value.map(_.data).mkString(":")
       val sources =
         (unmanagedSources in (Compile, compile)).value ++
           (unmanagedSources in (`dotty-compiler`, Compile)).value
@@ -375,7 +379,7 @@ object Build {
         "-project", "Dotty",
         "-project-version", dottyVersion,
         "-project-url", dottyGithubUrl,
-        "-classpath", s"$dottyLib:$dottyInterfaces:$otherDeps"
+        "-classpath", dottydocClasspath.value
       )
       (runMain in Compile).toTask(
         s""" dotty.tools.dottydoc.Main ${args.mkString(" ")} ${sources.mkString(" ")}"""
@@ -384,10 +388,7 @@ object Build {
 
     dottydoc := Def.inputTaskDyn {
       val args: Seq[String] = spaceDelimited("<arg>").parsed
-      val dottyLib = (packageAll in `dotty-compiler`).value("dotty-library")
-      val dottyInterfaces = (packageAll in `dotty-compiler`).value("dotty-interfaces")
-      val otherDeps = (dependencyClasspath in Compile).value.map(_.data).mkString(":")
-      val cp: Seq[String] = Seq("-classpath", s"$dottyLib:$dottyInterfaces:$otherDeps")
+      val cp: Seq[String] = Seq("-classpath",dottydocClasspath.value)
         (runMain in Compile).toTask(s""" dotty.tools.dottydoc.Main ${cp.mkString(" ")} """ + args.mkString(" "))
     }.evaluated,

- use File.pathSeparator instead of hard-coded path separator `:`
  => this way Windows user can run the command as well
 - reduce code duplication: move classpath string generation to
      own task and reuse it in genDocs and dottydocs
@robstoll robstoll force-pushed the genDocs-path-separator branch from 4e1042d to 655a80c Compare March 26, 2018 15:40
@robstoll
Copy link
Contributor Author

I have rebased the branch and made the improvement accordingly

Copy link
Contributor

@allanrenucci allanrenucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @robstoll! LGTM provided the tests pass

@allanrenucci allanrenucci merged commit d5ee92b into scala:master Mar 26, 2018
@robstoll robstoll deleted the genDocs-path-separator branch February 27, 2020 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants